Skip to content

feat(SERVE-RECIPE-ARGS): accept two inert recipe flags so a published vllm serve line reaches model load (#606) - #624

Merged
localai-bot merged 6 commits into
mainfrom
row/SERVE-RECIPE-ARGS
Aug 13, 2026
Merged

feat(SERVE-RECIPE-ARGS): accept two inert recipe flags so a published vllm serve line reaches model load (#606)#624
localai-bot merged 6 commits into
mainfrom
row/SERVE-RECIPE-ARGS

Conversation

@localai-bot

@localai-bot localai-bot commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

Closes #606. Also closes #621 (found in flow, fixed in the same change).

Builds on row/serve-recipe-args, which carries the committed spec. That
branch is not on main, so this PR contains its spec commit (0d8274ac3) plus
the implementation. Targeting main rather than the spec branch is deliberate:
the spec has no other consumer.

The defect

vllm-serve rejects any argument it does not recognise
(server_main.cpp's server: unknown argument '<flag>'). That is the right
default. It is also why copy-pasting an official vllm serve line aborted before
a single weight was read. Over vllm-project/recipes @ 86c7777a (157 model
recipes), --enable-auto-tool-choice appears in 89 and --trust-remote-code
in 82, and neither means anything to this engine, including for models we
ship token-exact and gated.

The seam

An enumerated accepted-and-inert table in ParseArgs, one entry per flag
carrying whether it takes a value and the reason it is inert.

Three properties, each with its own test:

  1. Not a catch-all. FindAcceptedInertArg returns nullptr for anything
    unlisted, so the existing abort is untouched — including for flags inert only
    because we LACK the capability. Silently swallowing --tensor-parallel-size
    would let a user believe they got tensor parallelism, which is the failure
    mode the seam exists to prevent. --language-model-only (No multimodal input limits: --language-model-only (43 recipes) is sugar over per-modality limits we do not have #607) is a real
    capability gap and is deliberately not here.
  2. Accepting is announced. One notice per accepted flag, naming itself and
    its reason, so a log reader learns the flag did nothing rather than inferring
    that it worked.
  3. Inert is not unvalidated. --enable-auto-tool-choice with
    --tool-call-parser none is refused, mirroring
    vllm/entrypoints/openai/cli_args.py:395 at the pin 555967922 and printing
    upstream's own sentence.

The divergence, stated rather than papered over

Upstream defaults --tool-call-parser to None; we default it to hermes. So
upstream's flag genuinely gates something and ours cannot. This PR does not
change that default, and the notice text says so explicitly rather than letting a
reader infer the two agree when the parser is omitted. Upstream's
test_enable_auto_choice_fails_with_enable_reasoning is therefore not mirrored
as a failing case — it fails upstream because the parser is unset, not because
reasoning conflicts, and mirroring the assertion would encode upstream's default
instead of upstream's behaviour. The spec records this.

Evidence

tests/vllm/entrypoints/openai/test_serve_recipe_args.cpp re-execs the test
binary into the real VllmServerMain. ParseArgs reports a bad argument
through Usage(), which calls std::exit, so the abort cases are unobservable
in-process. The model directory is deliberately nonexistent, which makes
"parsing succeeded and the engine reached load" visible (server: loading model from ...) without a checkpoint or a bound port.

Result
RED (final test vs the pre-change binary, scratch build) 4 cases | 1 passed | 3 failed, 58 assertions | 41 passed | 17 failed, Status: FAILURE!
GREEN 4 cases | 4 passed | 0 failed, 58 assertions | 58 passed | 0 failed, Status: SUCCESS!
Mutation (catch-all, scratch copy) exactly an unlisted unknown serve flag still aborts turns RED (8 assertions); the other three stay GREEN
Serve conformance test_openai_api_server, test_openai_conformance, test_serve_recipe_args — 3/3 passed
scripts/agent-preflight.sh --staged exit 0, All gates green

The one case that already passed before the change is the load-bearing guard: it
must hold both before and after, and the mutation is what proves the test
defends it rather than merely describing it.

Records

  • SERVE-RECIPE-ARGS moves SPIKE -> ACTIVE; area and total counts
    reconciled in the same edit (Serving SPIKE 1->0, ACTIVE 9->10; Total
    SPIKE 5->4, ACTIVE 29->30).
  • Not claimed: GATING (would require closing CLAIM-SERVE-RECIPE-ARGS, which
    this implementer does not own) or DONE (needs a fresh review, the operator's
    gate rerun, and a parity-ledger anchor).
  • The move puts the row in GATED_STATES for the first time, so
    RUNNABLE_BASELINE is re-pinned in this same change, naming the row and the
    reason, as that checker's own message demands.
  • The spec gained the structured sections a non-SPIKE state requires; its
    existing content is unchanged.
  • docs/USAGE.md, docs/STATUS.md, docs/BENCHMARKS.md updated. The
    BENCHMARKS entry records no number owed rather than a fabricated axis.

#621, in flow

docs/BENCHMARKS.md hardcoded check-gate-commands' pin size as "25 record
rows". The pin holds 33 and is designed to move in both directions, so that
sentence had been wrong since 2026-08-06 and this PR makes it one wronger.
AGENTS.md forbids storing a measurement of one file inside another, so the count
is deleted rather than refreshed — refreshing it would restore accuracy and
keep the defect. The paragraph is already at the public-doc prose budget, so the
reasoning lives in the commit message and in #621 rather than in the document.

CI at this head: three red lanes, none of them this diff

Named here rather than only in a thread comment, because a PR body that lists
some of the red lanes tells a reader the rest are green.

Lane Failure Attribution
windows-msvc-cpu, windows-msvc-vulkan test_openai_api_server.exe exited with status -1073740791 (0xC0000409, STATUS_STACK_BUFFER_OVERRUN) #584. Records-only #625 fails with the byte-identical status and touches no src/ or include/ path, so a PR that changes no C++ reproduces it. test_openai_api_server never calls ParseArgs, so it is not a consumer of this seam either.
build-test-cpu test_cpu_threadpool, 1 of 404: ratio 101.567 against CHECK( ratio < 100.0 ) #631. test_serve_recipe_args itself passed on that same runner.

On #631, to the same standard as #584 rather than a bare "unrelated". The guard
divides two wall-clock medians, over_us / fits_us, and compares the ratio to a
fixed 100, so both ends are machine-shape dependent — and the denominator is the
problem. Same commit, same code, three observations:

Box fits over ratio
2-core CI runner 2 threads, 0.48 us 5 threads, 48.752 us 101.567 RED
20-core box (#631's table) 10 threads, 7.213 us 21 threads, 19.467 us 2.699 GREEN
20-core box, rebuilt at the reviewed head 10 threads, 13.256 us 21 threads, 13.135 us 0.990872 GREEN

The third row was measured while repairing this record: test_cpu_threadpool
9 cases / 9 passed, 19602 assertions / 0 failed, Status: SUCCESS!. Two
same-class boxes 2.7x apart is itself the finding — the statistic will not carry
a fixed threshold. build-test-cpu is green on the scheduled main lane at
baseline 7572b0f4e, and this diff is argument parsing inside ParseArgs, which
has no path to a threadpool dispatch ratio. The test's own comment
(tests/vt/test_cpu_threadpool.cpp:496) claims it can "be trusted ... never to
fail spuriously on a busy one"; the CI run falsifies that.

Deliberately not fixed here: repairing that guard changes a gate's semantics,
which AGENTS.md routes through its own spec and red-before evidence, and raising
100 to a larger number would be widening a scope to turn a red gate green.

Fixed in flow and now green: pr-size (governance checkers lacked paired
mutation evidence) and the Windows portability gate (open ( inside a string
literal read as a POSIX call), both in b7eaadd07.

Review findings, repaired in 672881803

The fresh scoped review returned FAIL on the record, not the code — the seam,
the mirrored validation, the notice and the four tests all reproduced exactly,
mutation included, and it stated that no source change was required. No src/ or
tests/ behaviour changes in the repair.

  1. The unnamed red lane above, now in the spec's ## Now and in this body.
  2. A comment false about its own tree. check-agent-record.py's ENGINE_ROWS
    152 -> 153 justification said the row is SPIKE with "no seam, no test, and no
    docs/USAGE.md entry". True at the spec commit 0d8274ac3, false at this
    head. main is squash-only, so the single landing commit would have shipped a
    comment contradicted by files in the same commit. Corrected to state the tree.
  3. Three stale line anchors, re-derived from the file: FindAcceptedInertArg
    308 -> 312, the parse branch 501 -> 505, the mirrored validation
    557 -> 560, fixed in both .agents/engine-matrix.md and the spec.
    kAcceptedInertArgs:289 and all four test anchors were already exact.
  4. The roadmap_v1.md § Recipe-surface sweep citation is a forward reference
    to record(intake): place the recipes.vllm.ai sweep and its six issues (#605-#610) #612, which adds that section along with the intake rows for vllm-serve rejects 61 of 76 official-recipe --reasoning-parser uses, including qwen3 on our own gate models #605-Tool-call parsers: 5 upstream names missing (2 invisible to the recipe sweep), and the row records none of the 40 that exist #608. The
    citation now says so. vllm-serve aborts on --enable-auto-tool-choice and --trust-remote-code, so 89 of 157 official recipe commands fail to start #606 is deliberately not added to the intake table:
    record(intake): place the recipes.vllm.ai sweep and its six issues (#605-#610) #612 owns those keys, and two branches adding the same keyed record merge
    cleanly and then define it twice.
  5. InertArg::takes_value recorded as a named cold path. Both entries are
    kNoValue, so the value-consuming branch is unexercised; the spec now carries
    the obligation that the first entry with takes_value == true brings a test in
    the same change. No speculative code.

scripts/agent-preflight.sh --staged exit 0, "All gates green";
scripts/check-windows-portability.py exit 0 (Windows-only, not part of
preflight); check-pr-size exit 0; tests/scripts/test_agent_record.py 50 tests
OK.

For the reviewer

The mutation to reproduce is widening FindAcceptedInertArg into a catch-all.
It must turn an unlisted unknown serve flag still aborts RED and nothing else.
A weaker mutation worth trying: delete the --tool-call-parser none validation
and confirm the third case goes RED.

No GPU, no oracle run, no forward pass touched.

🤖 Generated with Claude Code

mudler added 4 commits August 13, 2026 10:37
…load (#606)

FOLLOWING_AGENTS_PROTOCOL

vllm-serve rejects any argument it does not recognise (server_main.cpp:440). That
is the right default, and it is why two flags that mean nothing to us stop the
server before it loads a model: --enable-auto-tool-choice, which 89 of the 157
official vLLM recipes pass, and --trust-remote-code, which 82 pass. We parse tool
calls whenever --tool-call-parser resolves, so there is no second gate to open;
and we have no Python, so there is no remote code to trust. Both are inert, and
both abort -- including for Qwen3.5-27B, which we ship token-exact and gated. The
kernels are not what stands between a user and a running server here.

The spec is deliberately narrow about what the seam may swallow. An enumerated
table, one entry per flag with the reason it is inert; anything unlisted still
aborts, because silently accepting --tensor-parallel-size would let a user believe
they got tensor parallelism, and that is the failure mode this exists to prevent.
Flags inert because we LACK the capability -- TP, EP, --mm-encoder-tp-mode -- are
explicitly excluded and keep aborting. So is --language-model-only (#607), which
is a real capability gap wearing the same costume.

Two things the spec records rather than papers over. cli_args.py:395 makes
--enable-auto-tool-choice without --tool-call-parser a TypeError upstream, so
inert must not mean unvalidated and the mirrored failure is a named test case. And
upstream defaults --tool-call-parser to None where we default it to hermes, so
upstream's flag genuinely gates something ours cannot -- pre-existing, out of
scope, and called out so the notice text does not overclaim parity.

ENGINE_ROWS 152 -> 153 for the new row, with the justification the checker's own
comment ledger asks for. No checker semantics change: the row claims no seam, no
test and no USAGE entry, because none exists yet.

Spec and records only, no product code touched.
scripts/agent-preflight.sh --staged: exit 0.

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:claude-opus-5 [Claude Code]
… vllm serve line reaches model load (#606)

`vllm-serve` rejects any argument it does not recognise. That is the right
default, and it is also why copy-pasting an official `vllm serve` command aborted
before a single weight was read. Over `vllm-project/recipes` @ `86c7777a` (157
model recipes), `--enable-auto-tool-choice` appears in 89 and
`--trust-remote-code` in 82, and neither means anything to this engine.

Both are now accepted and inert, from an ENUMERATED table in `ParseArgs`
(`kAcceptedInertArgs`), one entry per flag carrying whether it takes a value and
the reason it does nothing.

This is deliberately not a catch-all. `FindAcceptedInertArg` returns nullptr for
anything unlisted, so the existing `server: unknown argument '<flag>'` abort is
untouched, including for flags that are inert only because we LACK the
capability: silently swallowing `--tensor-parallel-size` would let a user believe
they got tensor parallelism, which is the failure mode the seam exists to
prevent. `--language-model-only` (#607) is a real capability gap and is not here.

Accepting is announced. Each accepted flag prints one notice naming itself and
its reason, so a log reader learns the flag did nothing rather than inferring
that it worked.

Inert is not unvalidated. `--enable-auto-tool-choice` with `--tool-call-parser
none` is refused, mirroring `vllm/entrypoints/openai/cli_args.py:395` at the pin
`555967922` and printing upstream's own sentence. `none` is our spelling of
upstream's falsy unset parser: upstream defaults `--tool-call-parser` to `None`
and we default it to `hermes`, a pre-existing divergence this change does not
touch and does not paper over. The notice text says so explicitly rather than
letting a reader infer the two agree when the parser is omitted.

RED-first. `tests/vllm/entrypoints/openai/test_serve_recipe_args.cpp` re-execs
the test binary into the REAL `VllmServerMain` (ParseArgs reports a bad argument
through `Usage()`, which calls `std::exit`, so the abort cases are unobservable
in-process) against a deliberately nonexistent model directory, which makes
"parsing succeeded and the engine reached load" visible without a checkpoint or a
bound port. Against the pre-change binary: 4 cases, 1 passed, 3 failed, 58
assertions, 17 failed, Status: FAILURE!. After: 58/58, Status: SUCCESS!.

Mutation-proven, not merely inspected. Widening `FindAcceptedInertArg` into a
catch-all in a scratch copy turns exactly one case RED, "an unlisted unknown
serve flag still aborts" (8 assertions), and leaves the other three green.

The row leaves SPIKE for ACTIVE, which puts it in `GATED_STATES` for the first
time, so `RUNNABLE_BASELINE` is re-pinned in this same change as its checker's
own message demands, naming the row and the reason. Not claimed: GATING, which
would require closing `CLAIM-SERVE-RECIPE-ARGS`, or DONE, which additionally
needs a fresh review, the operator's gate rerun and a parity-ledger anchor.

Also fixes #621, found in flow: `docs/BENCHMARKS.md` hardcoded
`check-gate-commands`' pin size as "25 record rows". The pin holds 33 and is
designed to move in both directions, so that sentence had been wrong since
2026-08-06 and this change makes it one wronger. AGENTS.md forbids storing a
measurement of one file inside another, so the count is deleted rather than
refreshed; refreshing it would restore accuracy and keep the defect. The
paragraph is at the public-doc prose budget, so the reasoning lives here and in
the issue rather than in the document.

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:claude-opus-5 [Claude Code]
…n, and two pin bumps owed mutation evidence (#606)

Three red CI checks on #624, all real, none of them the C++ change itself.

windows-msvc-cpu and windows-msvc-vulkan:

    src/vllm/entrypoints/openai/server_main.cpp:300: unguarded POSIX
    include/call reaches Windows

`check-windows-portability.py` scans the shipped server sources with COMMENTS
stripped but STRING LITERALS intact, and its POSIX-call pattern matches a bare
`open` before a parenthesis. The notice text read "...no second gate to open
(note --tool-call-parser defaults to hermes here...)", so the literal `open (`
inside a user-facing message read as a call to POSIX `open`.

The checker is not weakened, and the message is not silently dropped: the
parenthetical becomes a semicolon clause, which keeps every word of the caveat
the spec requires about our hermes default. A comment at the site says why, so
the next person to reach for parentheses there learns it from the code rather
than from a Windows runner twenty minutes later. This gate is Windows-only and
does not run in `agent-preflight.sh`, which is why it was not caught locally.

pr-size: two governance checkers changed without the mutation evidence their
contract demands. `check-gate-commands.py` gained the `RUNNABLE_BASELINE`
re-pin for this row; `check-agent-record.py` had `ENGINE_ROWS` bumped 152 -> 153
by the spec commit this branch builds on, which never faced a PR gate.

Both now carry a paired test, in the shape the neighbouring rows already use:

- `test_serve_recipe_args_row_is_inside_the_engine_ratchet` asserts the row and
  the ratchet bump are one semantic change, and pins WHICH matrix owns the row
  so a drift into another matrix cannot leave the engine count short while the
  pin still reads 153.
- `test_serve_recipe_args_is_credited_for_real_commands` asserts the row audits
  as `runnable` and that its spec names commands that can actually fail,
  including why no GPU or oracle leg is implicated.
- `test_dropping_serve_recipe_args_from_the_pin_breaks_it` mutates in the
  direction this re-pin actually moved: remove the entry and the exact-pin
  equality has to disagree with the audit. That is what proves the row was
  pinned because it entered the population, not to quiet a gate.

`docs/USAGE.md` now quotes the notice verbatim, so a reader can match the line
in their log against the table that explains it.

Re-verified: test_serve_recipe_args 4 cases / 58 asserts, Status: SUCCESS!.
scripts/agent-preflight.sh --staged: exit 0, all gates green.
scripts/check-windows-portability.py no longer reports any unguarded POSIX line.

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:claude-opus-5 [Claude Code]
…and document the refusal text (#606)

Self-review of the diff before handing it to a reviewer found two comment
defects. Neither changes behaviour; both would have misled the next reader, which
is the only thing a comment can get wrong.

The entry for `--enable-auto-tool-choice` said "the parenthetical is deliberate
and must stay". It was a parenthetical until the Windows POSIX scan forced the
`open (` rewrite one commit ago, and it is a semicolon clause now. The
instruction is still right and still load-bearing, so the wording is corrected
rather than the rule.

The mirrored `cli_args.py:395` check was inserted directly beneath the comment
that explains the parser-dialect validation, so that comment described code four
lines further down and appeared to describe mine. The check moves above it,
which restores the pairing and puts the two validations in the order that
actually matters: `none` is itself a registered selection, so the dialect check
passes on a contradictory pair and would say nothing about the conflict. Running
the mirrored check first means a contradiction is reported as a contradiction.
That ordering is now stated where it is enforced, and shown in `docs/USAGE.md`
with the exact two lines a user sees.

Re-verified after the change: test_serve_recipe_args 4 cases / 58 asserts,
Status: SUCCESS!. check-windows-portability.py reports no unguarded POSIX line.
scripts/agent-preflight.sh --staged: exit 0, all gates green.

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:claude-opus-5 [Claude Code]
@localai-bot

Copy link
Copy Markdown
Collaborator Author

CI note: windows-msvc-cpu / windows-msvc-vulkan are #584, not this change

Both Windows lanes fail with:

test_openai_api_server.exe exited with status -1073740791

-1073740791 is 0xC0000409, STATUS_STACK_BUFFER_OVERRUN, and it is already
tracked as #584 ("test_openai_api_server crashes on Windows with
STATUS_STACK_BUFFER_OVERRUN (0xC0000409), unmasked by the #512 fix").

Attributed rather than assumed: #625 fails identically, byte-for-byte the
same exit status, and its diff touches no src/ or include/ path at all
(.agents/model-matrix.md, .agents/roadmap_v1.md, docs/FEATURES.md,
scripts/check-agent-record.py, tests/scripts/test_agent_record.py). A
records-only PR cannot have caused a crash in a C++ test binary, so the failure
is on the base, not in this branch.

test_openai_api_server is also not a consumer of what this PR changes: the seam
lives in ParseArgs, which that test never calls. It links the same library, and
it crashed the same way before this branch existed.

The one Windows failure that was mine, and is fixed

The first run failed differently and legitimately:

src/vllm/entrypoints/openai/server_main.cpp:300: unguarded POSIX include/call reaches Windows

check-windows-portability.py scans the shipped server sources with comments
stripped but string literals intact, and its POSIX-call pattern matches a bare
open before a parenthesis. The notice text read ...no second gate to open (note --tool-call-parser defaults to hermes...), so open ( inside a user-facing
message read as a call to POSIX open. Fixed in b7eaadd07 by turning the
parenthetical into a semicolon clause — every word of the caveat is kept, the
checker is not weakened, and a comment at the site records the trap. That gate is
Windows-only and is not part of scripts/agent-preflight.sh, which is why it did
not surface locally.

Locally after the fix: check-windows-portability.py reports zero unguarded POSIX
lines, and test_serve_recipe_args is 4 cases / 58 assertions, Status: SUCCESS!.

…nd record the literal-scanning trap (#606)

Two things the next person on this row would otherwise re-derive, so the spec's
Now section states them.

The PR cannot go fully green, and not because of this change. windows-msvc-cpu
and windows-msvc-vulkan fail with test_openai_api_server.exe exiting
-1073740791, which is 0xC0000409, STATUS_STACK_BUFFER_OVERRUN, already tracked
as #584. Attributed rather than assumed: #625 fails with the byte-identical exit
status while touching no src/ or include/ path at all, so a records-only PR
reproduces it. test_openai_api_server is not a consumer of this seam either --
it never calls ParseArgs.

And the trap this change walked into. check-windows-portability.py scans the
shipped server sources with comments stripped but STRING LITERALS INTACT, and
its POSIX-call pattern matches a bare `open` before a parenthesis, so prose in a
user-facing message was reported as an unguarded POSIX call reaching Windows.
The gate is Windows-only and is not part of scripts/agent-preflight.sh, which
means an all-green local preflight says nothing about it. That is the part worth
writing down: the failure mode is not the rule being wrong, it is a gate the
local preflight does not cover.

Record only. No product code, no test, no checker touched.
scripts/agent-preflight.sh --staged: exit 0, all gates green.

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:claude-opus-5 [Claude Code]
@localai-bot

Copy link
Copy Markdown
Collaborator Author

CI attribution, part 2: build-test-cpu is #631, and my new test passed

build-test-cpu went red with 1 failure out of 404, and it is not this
change. My own test passed on the runner:

258/404 Test #258: test_serve_recipe_args ......   Passed    0.03 sec

The single failure is test_cpu_threadpool:

tests/vt/test_cpu_threadpool.cpp:536: MESSAGE: empty-op dispatch: 2 threads 0.48 us, 5 threads 48.752 us, ratio 101.567
tests/vt/test_cpu_threadpool.cpp:539: ERROR: CHECK( ratio < 100.0 ) is NOT correct!
  values: CHECK( 101.567 <  100 )

A threadpool scheduling-latency guard, 1.6% over its line. This PR's diff is
argument parsing in ParseArgs and cannot reach it.

Verified rather than asserted — same commit, same code, two machines:

Box fits over ratio
2-core CI runner 2 threads, 0.48 us 5 threads, 48.752 us 101.567 (RED)
this branch, 20-core dev box 10 threads, 7.213 us 21 threads, 19.467 us 2.699 (GREEN)

If this branch had caused it, my tree would fail. It passes with a 37x margin.
build-test-cpu is also green on the scheduled main lane at the newest
baseline 7572b0f4e2fb.

The guard divides two wall-clock medians and compares to a fixed 100; on a
2-core runner the denominator collapses to 0.48 us and ordinary noise moves the
ratio by tens. Filed as #631 with the reproduction, and deliberately not
fixed here: changing a gate's semantics needs its own spec and red-before
evidence, and raising the threshold would be widening a scope to turn a red gate
green.

Status of the three red checks on this PR

Check Cause Mine?
windows-msvc-cpu / -vulkan test_openai_api_server.exe exits 0xC0000409 No — #584, reproduced by records-only #625 with a byte-identical status
build-test-cpu test_cpu_threadpool ratio guard No — #631, passes locally on this branch at 2.699
pr-size governance checkers lacked paired mutation evidence Yes — fixed in b7eaadd07, now green
windows portability open ( in a string literal read as a POSIX call Yes — fixed in b7eaadd07, error gone from the log

Green on this SHA: agent-record, pr-size, build-test-vulkan,
build-test-cpu-arm64, both sanitize-cpu jobs, verify (cpu),
verify (vulkan), documentation-checkpoint, commit-protocol-tag,
device-leakage, cuda-arch-features, vulkan-spirv-freshness.

…ment and three anchors that disagree with the tree (#606, #631)

Repairs the fresh scoped review of #624. The review's verdict was FAIL on the
RECORD and explicitly not on the code: the seam, the mirrored cli_args.py:395
validation, the notice and the four tests all reproduced exactly, mutation
included. Nothing in `src/` or `tests/` changes here, by design.

1. AN UNNAMED RED LANE (the one that matters). The spec's `## Now` attributed
   the red CI to the two `windows-msvc-*` lanes and #584, so a reader was told
   the CPU lane was green. It is not: `build-test-cpu` fails on
   `test_cpu_threadpool`, 1 of 404, at `ratio 101.567` against a fixed `< 100.0`.
   AGENTS.md § Gates wants exactly one result per applicable rule, and a lane
   omitted reads as a lane satisfied. It is now named, with the same attribution
   rigor already spent on #584 rather than a bare "unrelated": the guard divides
   two wall-clock medians, the 2-core runner collapsed the DENOMINATOR to
   0.48 us, and argument parsing inside `ParseArgs` has no path to a threadpool
   dispatch ratio. Already filed as #631 while the review was running, so this
   references that issue rather than opening a second one.

   The evidence is three observations of the same code, and the third is the
   informative one. The 2-core runner read 101.567; #631's table read 2.699 on a
   20-core box; rebuilding the reviewed head here read 0.990872 on a 20-core box
   (9 cases / 9 passed, 19602 assertions, `Status: SUCCESS!`). Two same-class
   boxes 2.7x apart is the finding: the statistic will not carry a fixed
   threshold. Recorded rather than smoothed, and the middle row is kept
   attributed to #631 rather than restated as mine. Also recorded because it
   falsifies the test's own claim at `test_cpu_threadpool.cpp:496` that it can
   "be trusted ... never to fail spuriously on a busy one".

   NOT fixed in flow, deliberately. Repairing that guard changes a gate's
   semantics, which AGENTS.md routes through its own spec and red-before
   evidence; and raising 100 to a larger number would be widening a scope to
   turn a red gate green, which is the specific thing forbidden.

2. A COMMENT FALSE ABOUT ITS OWN TREE. The `ENGINE_ROWS` 152 -> 153 justification
   in `check-agent-record.py` said the row is `SPIKE` and that "no seam, no test,
   and no `docs/USAGE.md` entry exists yet". All four were true at the spec
   commit `0d8274ac3` and none is true at this head. `main` is squash-only, so
   the single landing commit would have shipped a comment contradicted by files
   in the same commit — AGENTS.md § History is git: the record cannot disagree
   with the tree.

   Corrected to state the tree rather than framed "as of the spec commit". The
   as-of framing would also have been accurate, and is rejected on purpose: it
   dates a justification to a commit that squash-merging deletes, so the reader
   who meets the line has no way to check it. A comment that describes the file
   it sits in is verifiable by looking; one that describes a vanished
   intermediate is not.

3. THREE STALE LINE ANCHORS, in `.agents/engine-matrix.md` and the spec.
   `b7eaadd07` inserted a 4-line comment at `server_main.cpp:299-302` and shifted
   everything below it. Re-derived from the file rather than from the review's
   table, and the table was right: `FindAcceptedInertArg` 308 -> 312, the parse
   branch 501 -> 505, the mirrored validation 557 -> 560. `kAcceptedInertArgs:289`
   sits above the insertion and all four test anchors (`:141`, `:165`, `:188`,
   `:212`) are exact; those five are untouched.

4. A FORWARD REFERENCE, made legible instead of resolved. The spec cited
   `roadmap_v1.md` § Recipe-surface sweep, which does not exist there yet — it
   arrives with #612, along with the intake rows for #605-#608. The citation now
   says so. #606 is deliberately NOT added to the intake table: #612 owns those
   keys, and two branches adding the same keyed record merge cleanly and then
   define it twice, which has already happened twice in this campaign.

5. `InertArg::takes_value` recorded as a named cold path. Both shipped entries
   are `kNoValue`, so the value-consuming branch is unexercised and cannot be
   tested without inventing an entry. The obligation moves to the spec: the first
   entry with `takes_value == true` owes a test in the same change. No
   speculative code and no speculative test.

check-pr-size requires paired mutation evidence for any change to
`scripts/check-agent-record.py`. This PR already carries it —
`test_agent_record.py`'s `test_serve_recipe_args_row_is_inside_the_engine_ratchet`
is the semantic evidence for the 152 -> 153 bump, and the checker verifies the
pair over the whole PR range. The edit above is comment-only and changes no
behavior that any test could newly pin, so no test is added: a test written to
satisfy a counter rather than to defend a guarantee is the thing that rule
exists to prevent.

Verification, all foreground and observed: `scripts/agent-preflight.sh --staged`
exit 0, "All gates green"; `scripts/check-windows-portability.py` exit 0,
"Windows portability contract OK" (it is Windows-only and not part of preflight,
which is how the earlier `open (` trap got through); each corrected anchor read
back from the file it points into.

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:claude-opus-5 [Claude Code]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants